Skip to content

fix(sync): harden skill scanning against bad paths, reject filesystem-root plugin sources#440

Merged
christso merged 1 commit into
mainfrom
fix/433-skill-scan-eperm
Jul 15, 2026
Merged

fix(sync): harden skill scanning against bad paths, reject filesystem-root plugin sources#440
christso merged 1 commit into
mainfrom
fix/433-skill-scan-eperm

Conversation

@christso

Copy link
Copy Markdown
Contributor

Summary

A user reported that npx allagents plugin uninstall <plugin> printed ✓ Uninstalled plugin (user scope): ... and then crashed with Error: EPERM: operation not permitted, scandir 'C:\$WinREAgent\Scratch\Mount\Windows\System32\LogFiles\WMI\RtBackup'. They were already on v1.13.2 (past the #434 getHomeDir() fix), so this was a different bug.

Root cause: their ~/.allagents/workspace.yaml had a corrupted plugin entry — a bare \ — which resolves to the drive root on Windows. walkForSkillMd (the recursive skill scanner in src/core/skills.ts) had no try/catch and no symlink guard (unlike its sibling discoverRepoSkills in repo-skills.ts), so it recursed unguarded into C:\ and crashed the entire post-uninstall sync — even though the uninstall itself had already succeeded, and even though the bad plugin wasn't the one being uninstalled (sync scans every installed plugin).

  • walkForSkillMd hardening (src/core/skills.ts): catches readdir errors and records a warning with remediation steps instead of throwing; skips symlinks/junctions to avoid unbounded recursion through loops. Warnings thread through collectPluginSkillscollectAllSkills/collectAvailableSkillNamesSyncResult.warnings, deduplicated since the same bad plugin gets scanned twice internally.
  • Entry-point validation (src/utils/plugin-path.ts, workspace-modify.ts, user-workspace.ts): addPlugin/addUserPlugin now reject a local-path source that resolves to a filesystem root (new isFilesystemRoot helper) — closing the entry point that let the bare \ get written in the first place. There's no legitimate use case for a plugin source that resolves to a drive/filesystem root.
  • Fixed a related silent gap: runUserSyncAndPrint (plugin.ts) never printed the Warnings: section at all (unlike its project-scope sibling runSyncAndPrint) — user-scope syncs were silently swallowing every warning, not just the new ones.

Test plan

  • bun test — new/extended unit tests in skills.test.ts, plugin-path.test.ts, sync-resilient.test.ts, workspace-modify.test.ts, user-workspace.test.ts all pass
  • bun run typecheck and bun run lint clean
  • Manual E2E against the built CLI (bun run build, node dist/index.js ...):
    • Temp workspace with a good plugin + a plugin whose resolved path is unreadable (a file standing in for the real bug's drive-root path) → plugin uninstall <good-plugin> --scope user succeeds, sync completes (exit 0), and prints exactly one deduplicated warning with remediation steps instead of crashing.
    • plugin install "\" (both project and user scope) → rejected with Plugin source cannot be a filesystem root directory: \, exit 1, workspace.yaml untouched.

…-root plugin sources

A user hit `EPERM: operation not permitted, scandir 'C:\$WinREAgent\...'`
right after a successful `plugin uninstall`, on v1.13.2 (already past the
#434 getHomeDir() fix). Root cause: their workspace.yaml had a corrupted
plugin entry that was a bare `\`, which resolves to the drive root on
Windows — walkForSkillMd (skills.ts) recursed into it unguarded (no
try/catch, no symlink guard) and crashed the whole post-uninstall sync,
even though uninstall itself had already succeeded.

- walkForSkillMd now catches readdir errors and records a warning with
  remediation steps instead of throwing; also skips symlinks/junctions to
  avoid unbounded recursion through loops. Warnings thread through
  collectPluginSkills -> collectAllSkills/collectAvailableSkillNames ->
  SyncResult.warnings, deduplicated since the same bad plugin is scanned
  twice internally.
- addPlugin/addUserPlugin now reject a local-path source that resolves to
  a filesystem root (new isFilesystemRoot helper), closing the entry point
  that let the bare `\` get written in the first place.
- runUserSyncAndPrint (plugin.ts) was silently never printing the
  Warnings: section at all (unlike its project-scope sibling) - user-scope
  syncs were swallowing every warning, not just the new ones. Fixed to
  match runSyncAndPrint.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying allagents with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5dffc40
Status: ✅  Deploy successful!
Preview URL: https://1d429232.allagents.pages.dev
Branch Preview URL: https://fix-433-skill-scan-eperm.allagents.pages.dev

View logs

@christso christso merged commit 1fdd768 into main Jul 15, 2026
5 checks passed
@christso christso deleted the fix/433-skill-scan-eperm branch July 15, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant